We’re already iterating the module routes in line 48, so we could just combine collecting all “system routes” with setting up the normal route handler here, to avoid iterating everything twice.

I initially tried to do that but i need to loop to get all routes before applying the 405 handler which is global whereas the route handler is per route

I was thinking about something like this:

foreach (var route in …)
{
    routeBuilder.MapVerb(…);
    knownRoutes.Add(route);
}

builder.UseRouter(routeBuilder.Build());
builder.Use(GetMethodNotAllowedHandler(knownRoutes));

If I understand it correctly, then this handler is supposed to “pick up” those routes which paths are registered but are using the incorrect verb? In that case, wouldn’t it be better to run this middleware after the normal mapped routes, so it only runs when handling those is over (similarly how in standard ASP.NET, MVC runs after StaticFiles if StaticFiles could not respond to a request).

If it runs after, the routing middleware will set 404 as the status code, the issue is, how do you know its a 404 from an unfound route vs a 404 wrongly assigned for invalid verb on path?

Unless I’m completely wrong (routing is one of the topics I didn’t really do anything with yet) but from how it looks, the next middleware is only invoked if there is no matching route. So if the middleware is after the router, it will only run for unrecognized routes. So you would only have to check whether any route with such a path exists in order to determine whether it’s a real not found or just an incorrect verb. So you could do this with a simple hash set on the paths.

If you’re looking up routes by paths, then you shouldn’t use a list. Consider an ILookup<string, string> instead that maps from paths to the registered verbs.

Per above: If this would run after the normal handlers as a fallback, then at this point you would be already guaranteed that this is running on an unsupported method. So you wouldn’t actually need to lookup (or even store) the registered methods but just the paths.

Not sure what David would say about this (allocations!?!?!), but maybe just route.path.TrimEnd('/')?

It might be cheaper to leave off the leading slash here, and trim the leading slash from the current request later instead.

Since this does not return a handler function (which is what I was originally suggesting) but is the handler, the Get feels out of place here. HandleMethodNotAllowed? Or maybe you should just move this out to a custom middleware class.

Yeah tried your approach but no joy

Why are you continuing the pipeline here at the beginning? Shouldn’t this handler already be kind of the final one in the pipeline? Do you need to know that it produced a 404? Wouldn’t it be safe to assume that if this handler runs after the routing middleware, then a route was not found? (again: I have no actual idea about the routing middleware)

Oddly before that is called the response status code is 200, if you call next it's then 404 from the router. Not sure how because as you say I'd expect it to be already at the end of the pipeline

I don’t think you actually need to check the status code here though. This should not run if the router succeeded in the pipeline step before.

Interestingly if I remove the next() call it will always be 200 on a route not defined in Botwin. If I take the GetMethodNotAllowedHandler out completely I get a 404 so I assume there must be one last pipeline called somewhere

Use HashSet or Dictionary for constant time lookup. Also, I don’t know if the router middleware is case insensitive itself but if it is, then pure string equality will not work (you will have to use the RouteComparer we came up with earlier).